tests: use `inline_snapshot.Is` on parametrized test by Kludex · Pull Request #945 · modelcontextprotocol/python-sdk

Kludex · 2025-06-12T10:35:50Z

cc @agronholm

This PR just uses inline_snapshots properly on a parametrized test.

agronholm · 2025-06-12T10:42:24Z

Looks like the tests are still failing though?

agronholm · 2025-06-12T10:54:32Z

That one failure looks like just a flaky test.

Kludex · 2025-06-12T11:01:20Z

Now the test is fixed... But it's failing correctly... 😅

The AnyUrl adds a very annoying trailing slash. @Viicos how is that going on the Pydantic side?

Viicos · 2025-06-12T11:05:14Z

The AnyUrl adds a very annoying trailing slash. @Viicos how is that going on the Pydantic side?

In progress in pydantic/pydantic-core#1719, but anyway this will only be included in 2.12.

agronholm · 2025-06-12T11:20:09Z

Now the test is fixed... But it's failing correctly... 😅

The AnyUrl adds a very annoying trailing slash. @Viicos how is that going on the Pydantic side?

If you access the MCP server without the trailing slash, it will redirect you to a URL with the slash added.

Kludex · 2025-06-12T11:23:08Z

Now the test is fixed... But it's failing correctly... 😅
The AnyUrl adds a very annoying trailing slash. @Viicos how is that going on the Pydantic side?

If you access the MCP server without the trailing slash, it will redirect you to a URL with the slash added.

Are you saying this is working as expected?

agronholm · 2025-06-12T12:12:05Z

Now the test is fixed... But it's failing correctly... 😅
The AnyUrl adds a very annoying trailing slash. @Viicos how is that going on the Pydantic side?

If you access the MCP server without the trailing slash, it will redirect you to a URL with the slash added.

Are you saying this is working as expected?

I have no idea how the test is supposed to work, TBH. I just observed a lot of redirects happening in the test suite when the forward slash is missing from the end of the URI.

Kludex · 2025-06-12T12:13:32Z

It's a Starlette thing. There's another PR that fixes the redirect behavior.

medaminezghal · 2025-06-16T18:21:15Z

@Kludex With pytest 8.4.0 I got this problem:

_________________________ ERROR at setup of TestOAuthClientProvider.test_generate_code_verifier __________________________
[gw4] linux -- Python 3.13.3 /home/medaminezghal/Documents/AUR_Packages/python-mcp/test/src/mcp-1.9.4/test-env/bin/python

cls = <class '_pytest.runner.CallInfo'>, func = <function call_and_report.<locals>.<lambda> at 0x7f3b3f42cf40>
when = 'setup', reraise = (<class '_pytest.outcomes.Exit'>, <class 'KeyboardInterrupt'>)

    @classmethod
    def from_call(
        cls,
        func: Callable[[], TResult],
        when: Literal["collect", "setup", "call", "teardown"],
        reraise: type[BaseException] | tuple[type[BaseException], ...] | None = None,
    ) -> CallInfo[TResult]:
        """Call func, wrapping the result in a CallInfo.
    
        :param func:
            The function to call. Called without arguments.
        :type func: Callable[[], _pytest.runner.TResult]
        :param when:
            The phase in which the function is called.
        :param reraise:
            Exception or exceptions that shall propagate if raised by the
            function, instead of being wrapped in the CallInfo.
        """
        excinfo = None
        instant = timing.Instant()
        try:
>           result: TResult | None = func()
                                     ^^^^^^

/usr/lib/python3.13/site-packages/_pytest/runner.py:344: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
/usr/lib/python3.13/site-packages/_pytest/runner.py:246: in <lambda>
    lambda: runtest_hook(item=item, **kwds), when=when, reraise=reraise
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
/usr/lib/python3.13/site-packages/pluggy/_hooks.py:512: in __call__
    return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
/usr/lib/python3.13/site-packages/pluggy/_manager.py:120: in _hookexec
    return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
/usr/lib/python3.13/site-packages/_pytest/logging.py:843: in pytest_runtest_setup
    yield
/usr/lib/python3.13/site-packages/_pytest/capture.py:895: in pytest_runtest_setup
    return (yield)
            ^^^^^
/usr/lib/python3.13/site-packages/_pytest/runner.py:164: in pytest_runtest_setup
    item.session._setupstate.setup(item)
/usr/lib/python3.13/site-packages/_pytest/runner.py:514: in setup
    col.setup()
/usr/lib/python3.13/site-packages/_pytest/python.py:1673: in setup
    self._request._fillfixtures()
/usr/lib/python3.13/site-packages/_pytest/fixtures.py:719: in _fillfixtures
    item.funcargs[argname] = self.getfixturevalue(argname)
                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
/usr/lib/python3.13/site-packages/_pytest/fixtures.py:548: in getfixturevalue
    fixturedef = self._get_active_fixturedef(argname)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
/usr/lib/python3.13/site-packages/_pytest/fixtures.py:639: in _get_active_fixturedef
    fixturedef.execute(request=subrequest)
/usr/lib/python3.13/site-packages/_pytest/fixtures.py:1127: in execute
    result = ihook.pytest_fixture_setup(fixturedef=self, request=request)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
/usr/lib/python3.13/site-packages/pluggy/_hooks.py:512: in __call__
    return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
/usr/lib/python3.13/site-packages/pluggy/_manager.py:120: in _hookexec
    return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
/usr/lib/python3.13/site-packages/pluggy/_callers.py:53: in run_old_style_hookwrapper
    return result.get_result()
           ^^^^^^^^^^^^^^^^^^^
/usr/lib/python3.13/site-packages/pluggy/_callers.py:38: in run_old_style_hookwrapper
    res = yield
          ^^^^^
/usr/lib/python3.13/site-packages/pluggy/_callers.py:53: in run_old_style_hookwrapper
    return result.get_result()
           ^^^^^^^^^^^^^^^^^^^
/usr/lib/python3.13/site-packages/pluggy/_callers.py:38: in run_old_style_hookwrapper
    res = yield
          ^^^^^
/usr/lib/python3.13/site-packages/_pytest/setuponly.py:36: in pytest_fixture_setup
    return (yield)
            ^^^^^
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

fixturedef = <FixtureDef argname='oauth_provider' scope='function' baseid='tests/client/test_auth.py'>
request = <SubRequest 'oauth_provider' for <Function test_generate_code_verifier>>

    def pytest_fixture_setup(
        fixturedef: FixtureDef[FixtureValue], request: SubRequest
    ) -> FixtureValue:
        """Execution of fixture setup."""
        kwargs = {}
        for argname in fixturedef.argnames:
            kwargs[argname] = request.getfixturevalue(argname)
    
        fixturefunc = resolve_fixture_function(fixturedef, request)
        my_cache_key = fixturedef.cache_key(request)
    
        if inspect.isasyncgenfunction(fixturefunc) or inspect.iscoroutinefunction(
            fixturefunc
        ):
            auto_str = " with autouse=True" if fixturedef._autouse else ""
    
>           warnings.warn(
                PytestRemovedIn9Warning(
                    f"{request.node.name!r} requested an async fixture "
                    f"{request.fixturename!r}{auto_str}, with no plugin or hook that "
                    "handled it. This is usually an error, as pytest does not natively "
                    "support it. "
                    "This will turn into an error in pytest 9.\n"
                    "See: https://docs.pytest.org/en/stable/deprecations.html#sync-test-depending-on-async-fixture"
                ),
                # no stacklevel will point at users code, so we just point here
                stacklevel=1,
            )
E           pytest.PytestRemovedIn9Warning: 'test_generate_code_verifier' requested an async fixture 'oauth_provider', with no plugin or hook that handled it. This is usually an error, as pytest does not natively support it. This will turn into an error in pytest 9.
E           See: https://docs.pytest.org/en/stable/deprecations.html#sync-test-depending-on-async-fixture

/usr/lib/python3.13/site-packages/_pytest/fixtures.py:1181: PytestRemovedIn9Warning

I think it's better to edit it to make it compatible with newer pytest versions.

agronholm · 2025-06-23T10:38:23Z

@medaminezghal my Trio PR (#946) fixes that too. Right now this PR is the only thing in this code base standing between me and a fully passing test suite.

agronholm · 2025-06-23T11:10:13Z

That SSE failure could be due to improperly nested async context managers (typically a stream closed before the task using it has finished)

tests/client/test_auth.py

felixweinberger

Approving to unblock #945 - added a proposal making the test an xfail on top of this branch in #1138 if you want to take a look @Kludex?

Kludex · 2025-07-14T14:14:45Z

Looks good.

felixweinberger · 2025-07-14T16:57:46Z

took the liberty of rebasing - looks like test_auth has changed significantly since this PR

Kludex mentioned this pull request Jun 12, 2025

Fix building auth metadata paths #779

Merged

9 tasks

Kludex requested a review from ihrpr June 13, 2025 08:38

dsp-ant self-requested a review June 17, 2025 09:47

agronholm mentioned this pull request Jun 23, 2025

Improved Trio support #946

Draft

9 tasks

ihrpr reviewed Jul 8, 2025

View reviewed changes

tests/client/test_auth.py Outdated Show resolved Hide resolved

ihrpr mentioned this pull request Jul 8, 2025

Unit Test Fix - Update concurrency test duration threshold and logging for clarity #878

Closed

9 tasks

felixweinberger previously approved these changes Jul 14, 2025

View reviewed changes

felixweinberger dismissed their stale review via 4a6fbd3 July 14, 2025 14:26

felixweinberger previously approved these changes Jul 14, 2025

View reviewed changes

Kludex added 4 commits July 14, 2025 17:50

tests: use inline_snapshot.Is on parametrized test

fdcaa57

Drop Is

a2a0d57

update again

a1a4b3a

Add comment and ignore test

c40ada6

felixweinberger dismissed their stale review via 62f114f July 14, 2025 16:57

felixweinberger force-pushed the fix-test-with-snapshot branch from 4a6fbd3 to 62f114f Compare July 14, 2025 16:57

tests: mark trailing slash test as xfail (#1138)

84937d1

felixweinberger force-pushed the fix-test-with-snapshot branch from 62f114f to 84937d1 Compare July 14, 2025 17:06

felixweinberger approved these changes Jul 14, 2025

View reviewed changes

ihrpr merged commit 95b44fb into main Jul 14, 2025
13 checks passed

ihrpr deleted the fix-test-with-snapshot branch July 14, 2025 20:59

Conversation

Kludex commented Jun 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

agronholm commented Jun 12, 2025

Uh oh!

agronholm commented Jun 12, 2025

Uh oh!

Kludex commented Jun 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Viicos commented Jun 12, 2025

Uh oh!

agronholm commented Jun 12, 2025

Uh oh!

Kludex commented Jun 12, 2025

Uh oh!

agronholm commented Jun 12, 2025

Uh oh!

Kludex commented Jun 12, 2025

Uh oh!

medaminezghal commented Jun 16, 2025

Uh oh!

agronholm commented Jun 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

agronholm commented Jun 23, 2025

Uh oh!

Uh oh!

felixweinberger left a comment

Choose a reason for hiding this comment

Uh oh!

Kludex commented Jul 14, 2025

Uh oh!

felixweinberger commented Jul 14, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Kludex commented Jun 12, 2025 •

edited

Loading

Kludex commented Jun 12, 2025 •

edited

Loading

agronholm commented Jun 23, 2025 •

edited

Loading